Skip to content

Conversation

@blob79
Copy link

@blob79 blob79 commented Oct 31, 2025

This implements unimpaired mappings:

[f Go to the file preceding the current one alphabetically in the current file's directory.
]f Go to the file succeeding the current one alphabetically in the current file's directory.

Comment on lines 381 to 382
| `['` | Go to the next file alphabetically in the current file's directory | `goto_next_file` |
| `]'` | Go to the previous file alphabetically in the current file's directory | `goto_prev_file` |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

['/]' could alternatively be nice to use for a feature that goes to the next/prev entry in the last picker (<space>'). I'm not sure what to recommend for the binding in ]/[ for this feature...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm picking [n, ]n.

}

/// Get the parent dir if it exists.
fn parent_dir(editor: &mut Editor) -> Option<PathBuf> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor style thing: I'd prefer that this live in helix-term/src/commands.rs rather than top-level here, since it's only used as a helper for a few commands

Comment on lines 6978 to 6987
let found = editor
.documents
.values()
.find(|doc| doc.path().map_or(false, |p| p == file))
.map(|doc| doc.id());
if let Some(id) = found {
editor.switch(id, Action::Replace);
} else if let Err(e) = editor.open(file, Action::Replace) {
editor.set_error(format!("Open file failed: {:?}", e));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editor::open already looks through open documents and switches if the document exists, so this block can be replaced with just the call to Editor::open

Some(value) => value,
None => return,
};
let (_, doc) = current_ref!(editor);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let (_, doc) = current_ref!(editor);
let doc = doc!(editor);

goto_next_file_impl(cx.editor, Direction::Backward, cx.count());
}

fn goto_next_file_impl(editor: &mut Editor, direction: Direction, count: usize) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nicer to have this function (and parent_dir) return an anyhow::Result. Then the typed commands can return that as-is. And the other callers can say:

if let Err(err) = goto_next_file_impl(...) {
    editor.set_error(err);
}

`[n` and `]n` implement the unimpaired mappings:

> [f                      Go to the file preceding the current one alphabetically in the current file's directory.
> ]f                      Go to the file succeeding the current one alphabetically in the current file's directory.

that are already taken.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants